Fix Windows environment set issue and change to env to params for compatibility #2
Conversation
Exelord
left a comment
Exelord
left a comment
There was a problem hiding this comment.
Thank you very much! ❤️ I didn't noticed this change. I have one feedback though :)
index.js
Outdated
| path.join('node_modules', '.bin', 'sentry-cli'), | ||
| url ? `--url ${url}` : '', | ||
| `--auth-token ${authToken}`, | ||
| 'releases', |
There was a problem hiding this comment.
I preferred to have releases in every command. sentryCliExec is meant to execute sentry cli not specific releases commands. :) Could you please revert that?
There was a problem hiding this comment.
With -- parameters you can't do that.
sentry-cli has a certain order rule for that. --org and --project parameters must always come after command.
At first I was going to write like this. this.sentryCliExec('release', new ${releaseName});
Two arguments first command, second params. That's why it left named params.
index.js
Outdated
| }, | ||
|
|
||
| sentryCliExec(command) { | ||
| sentryCliExec(params) { |
There was a problem hiding this comment.
We should get back to command :/ Params would an array or object of options.
There was a problem hiding this comment.
If this.sentryCliExec('release', new ${releaseName}); style ok to you. I'll change the signature with as sentryCliExec(command, params)?
|
How about now? I think this is better. |
2 participants
Hi.
All environment values changed to -- params for compatibility reasons.
I've tested on my project (on Windows) and working as expected. ( Except --url param which I don't have but I assume it'll work.)
Thank you for this great addon.